Add option to fetch lightweight objects to reduce app hangs #15774

itsmeichigo · 2025-06-18T10:00:36Z

Description

There have been warnings from Xcode about performing I/O on the main thread causing app hangs. Xcode Organizer also shows reports about app hangs related to this:

The stack traces point to ResultsController.fetchedObjects and ResultsController.object(at:), which then triggered Product.toReadOnly() and Order.toReadOnly in different areas of the app. These are heavy objects with complicated relationships that require multiple trips to the database file to fetch all data. Since we're doing all the fetches on the main thread, fetching multiple such heavy objects can cause app hangs.

This PR attempts to mitigate the issue by introducing the option to fetch lightweight objects - i.e., fetching objects without populating all their relationships. These are helpful for list views and other scenarios when it's not necessary to retrieve all the details of objects. This reduces the overhead of loading objects and thus minimizes app hangs.

Updated areas (following reports on Xcode Organizer and Xcode warnings):

Loading order details: OrderDetailsDataSource.reloadSections()
Checking shipping labels eligibility: OrderDetailsViewModel.localRequirementsForShippingLabelsAreFulfilled()
Checking for published products on order list: OrderListViewModel.hasAnyPublishedProducts
Blaze campaign creation form
Dashboard check for order count
New shipping label purchase form: WooShippingItemsDataSource
Product list

Testing steps

Confirm that you can no longer see warnings about performing I/O on the main thread on Xcode while running the following test cases. It's not predictable when Xcode shows the app hang warnings, so we may need to wait for app hang reports after releasing the app. For now, we can do regression testing to ensure that the app works correctly with the new changes.

TC1: Loading order details

Navigate to the Orders tab
Select a non-refunded order and confirm that the order items are listed correctly.
Select a refunded order and confirm that the details are displayed correctly.

TC2: Shipping Labels

Pre-requisite: Install the WooShipping extension on your test store and log in to the same store on the app as the store owner.
Navigate to the Orders tab and select a paid order with at least one physical product.
Confirm that the Create shipping label button is available.
Select Create shipping label button.
Confirm that the purchase form displays the correct products.

TC3: Test order entrypoint

Create a test store if you don't already have access to a store without any published products.
Enable cash on delivery payment for the store if it doesn't have any payment gateways yet.
Ensure that the store is public.
Log in to that test store on the app and navigate to the Orders tab. Confirm that the empty state screen has a button to create a test order.

TC4: Blaze campaign creation form

Log in to a WPCom store or install Blaze extension to your self-hosted store.
Navigate to the Products tab and select any published product.
Select Promote with Blaze.
Confirm that the campaign creation form displays the correct product image and relevant tag line and description for the product.
Select Ad Destination and confirm that the product URL is correct for the product.

TC5: Dashboard order count

Log in to a test store with no existing order.
Confirm that the Performance and Top Performers cards are not available on the dashboard and in the Dashboard > Edit list.
Create a new order in the store.
Confirm that the Performance and Top Performers cards are now available.

TC6: Product list

Build and run the app on an iPad
Log in to a test store with existing products.
Navigate to the Products tab.
Confirm that the first product on the list is selected automatically. Confirm that the details displayed on the product form are correct.
Confirm that the product list displays correct details for the items.

Testing information

Screenshots

No UI changes.

I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

wpmobilebot · 2025-06-18T10:10:04Z

📲 You can test the changes from this Pull Request in WooCommerce iOS Prototype by scanning the QR code below to install the corresponding build.

	App Name	WooCommerce iOS Prototype
	Build Number	`30609`
	Version	`PR #15774`
	Bundle ID	`com.automattic.alpha.woocommerce`
	Commit	`c65ddb4`
	Installation URL	4pntvkqvck2bo

Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

itsmeichigo · 2025-06-19T05:49:15Z

@jaclync I could not see the warnings regarding the product list as you reported, so could you please verify if this PR fixes that for you?

RafaelKayumov

I can see the I/O warning too. I guess that's because there are still code paths utilizing the "heavy" version of the toReadOnly. Putting print logs both into the toReadOnly and toLightweightReadOnly for Product and Order correspondingly reveal that the old method is still extensively used.

Mostly I see it in Order List tab.

I logged timings both for the heavy and lightweight readonly conversion. Looks like the light version works roughly ~10x faster. Is it a good trade-off for the fact that the "light" stripped down versions are the same "plain" objects, but just not containing relationships, and it's quite easy to treat them as complete objects and misuse them.

I wonder if we could also go for a more fundamental refactoring and make the toReadOnly conversion completely async, i.e. obtaining a Core Data object by NSManagedObjectID from a private context, doing conversion in a background and returning the result since the plain object is thread-safe? Though I doubt that we can afford it at the moment.

itsmeichigo · 2025-06-20T03:02:08Z

Thank you @RafaelKayumov for the review.

it's quite easy to treat them as complete objects and misuse them

I understand the concern. From my POV, this is similar to having a separate model that is used for list view versus a full model for a details screen. It's up to the developers to decide which to use and test their work.

I wonder if we could also go for a more fundamental refactoring and make the toReadOnly conversion completely async, i.e. obtaining a Core Data object by NSManagedObjectID from a private context, doing conversion in a background and returning the result since the plain object is thread-safe? Though I doubt that we can afford it at the moment.

This is one of the options that @jaclync proposed when she reported WOOMOB-619. I agree that it seems to be a more reliable solution, but it comes with the complexities of handling asynchronous logic, and requires refactoring how we use fetched objects all around the app. I'd prefer keeping the logic synchronous to be more expectable and easier to maintain.

Moreover, it seems like a waste of resources to try to get all the properties where they are not needed, so I chose the solution in this PR instead.

Mostly I see it in Order List tab.

Thanks for catching this! I didn't see the warning on my side. Added a change to the order cells to use the lightweight version of the objects in c65ddb4. Could you try again and see if the warning goes away?

joshheald · 2025-06-24T06:52:11Z

Thanks for taking the time to improve the performance here – the 10x improvement sounds significant.

it's quite easy to treat them as complete objects and misuse them

I understand the concern. From my POV, this is similar to having a separate model that is used for list view versus a full model for a details screen. It's up to the developers to decide which to use and test their work.

@itsmeichigo @RafaelKayumov I share this concern, because we've had similar problems with partially populated orders in the past. That time it happened because there were different _fields specified in different networking requests, so depending which part of the app had fetched and stored the Orders, they would display differently. I think in that case, the outcome was that people couldn't collect payment for orders.

In this PR, we stop handling order metadata (customFields: []) which could cause issues in corners of the app we don't use often. For example, card present refunds depend on a charge_id which comes from that metadata. Fortunately, we pull that out to a first-class property which is copied, so it'll be fine... but this makes the app less safe and harder to work on correctly.

With more AI-written code going in to the app, the chances of these kinds of errors increases too. Sure, testing our own work is important, but compiler safety is a better and stronger way to keep the app correct.

From my POV, this is similar to having a separate model that is used for list view versus a full model for a details screen.

That's a good analogy – but could we actually do that, instead of sending a half-empty full model? e.g.

public struct LightweightOrder {
    public let siteID: Int64
    public let orderID: Int64
    public let parentID: Int64
    public let customerID: Int64
    public let orderKey: String

    public let isEditable: Bool
    public let needsPayment: Bool
    public let needsProcessing: Bool
    public let number: String
    public let status: OrderStatusEnum
    public let currency: String
    public let currencySymbol: String
    public let customerNote: String

    public let dateCreated: Date
    public let dateModified: Date
    public let datePaid: Date?

    public let discountTotal: String
    public let discountTax: String
    public let shippingTotal: String
    public let shippingTax: String
    public let total: String
    public let totalTax: String
    public let paymentMethodID: String
    public let paymentMethodTitle: String
    public let paymentURL: URL?
    public let chargeID: String?

    public let billingAddress: Address?
    public let shippingAddress: Address?

    public let renewalSubscriptionID: String?
    public let attributionInfo: OrderAttributionInfo?
}

That way developers would be choosing a particular model that was needed, and forced to upgrade to the full model when they need it.

itsmeichigo · 2025-06-24T08:29:11Z

Thank you for your comment @joshheald. Given the concerns you shared, do you think it's better/safer to revert the changes in this PR and go through that new object route? Or can I/someone else follow up with a separate PR which is likely to be merged after 22.7 due to other priorities?

joshheald · 2025-06-24T08:43:17Z

Thank you for your comment @joshheald. Given the concerns you shared, do you think it's better/safer to revert the changes in this PR and go through that new object route? Or can I/someone else follow up with a separate PR which is likely to be merged after 22.7 due to other priorities?

It's safer to revert, IMO; keeping the app working comes ahead of improving performance, so that's what I'd do. However, it's your call, see what fits in best with your work and whether you think the risk is tolerable for a release cycle.

If you think you won't get it changed before 22.8, I'd definitely encourage you to revert this now. If it's the best way, you can always put it back when you have more time...

…15774)" This reverts commit 0bebc05, reversing changes made to b7e7ffd.

jaclync · 2025-06-24T10:15:57Z

Thanks for working on this, apology for not getting to this PR yesterday with a lot to catch up from AFK.

That's a good analogy – but could we actually do that, instead of sending a half-empty full model? e.g.

public struct LightweightOrder {
    public let siteID: Int64
    public let orderID: Int64
    public let parentID: Int64
    public let customerID: Int64
    public let orderKey: String

    public let isEditable: Bool
    public let needsPayment: Bool
    public let needsProcessing: Bool
    public let number: String
    public let status: OrderStatusEnum
    public let currency: String
    public let currencySymbol: String
    public let customerNote: String

    public let dateCreated: Date
    public let dateModified: Date
    public let datePaid: Date?

    public let discountTotal: String
    public let discountTax: String
    public let shippingTotal: String
    public let shippingTax: String
    public let total: String
    public let totalTax: String
    public let paymentMethodID: String
    public let paymentMethodTitle: String
    public let paymentURL: URL?
    public let chargeID: String?

    public let billingAddress: Address?
    public let shippingAddress: Address?

    public let renewalSubscriptionID: String?
    public let attributionInfo: OrderAttributionInfo?
}

That way developers would be choosing a particular model that was needed, and forced to upgrade to the full model when they need it.

This is what I had in mind when writing the issue as one of the potential solutions, a different struct with just the fields needed for the list view, instead of the full Product/Order struct with relationships not used in the list view. I agree that reusing the same struct and partially populating could cause unexpected issues and there is no easy way to know if it's the full version.

itsmeichigo added 3 commits June 18, 2025 12:55

Add simplified fetched objects to ResultsController

89a117d

Rename simplified to lightweight for better clarity

dd8bf1b

Use lightweight product objects where

c62beac

itsmeichigo added this to the 22.7 milestone Jun 18, 2025

itsmeichigo added feature: core Core work. See "category: tooling" and "category: architecture" Enhancement labels Jun 18, 2025

itsmeichigo added 3 commits June 19, 2025 11:14

Use lightweight readonly for checking for published products number

4df32c3

Use lightweight objects to check for order count on dashboard

d41e8b8

Merge branch 'trunk' into woomob-619-xcode-warnings-performing-io

5b4f590

itsmeichigo marked this pull request as ready for review June 19, 2025 05:43

Update release notes

a58151a

itsmeichigo requested review from jaclync and RafaelKayumov June 19, 2025 05:48

RafaelKayumov reviewed Jun 19, 2025

View reviewed changes

Merge branch 'trunk' into woomob-619-xcode-warnings-performing-io

321fe19

Use lightweight version for order list cells

c65ddb4

itsmeichigo requested a review from RafaelKayumov June 20, 2025 03:17

RafaelKayumov approved these changes Jun 20, 2025

View reviewed changes

itsmeichigo merged commit 0bebc05 into trunk Jun 24, 2025
13 checks passed

itsmeichigo deleted the woomob-619-xcode-warnings-performing-io branch June 24, 2025 03:12

itsmeichigo added a commit that referenced this pull request Jun 24, 2025

Revert "Add option to fetch lightweight objects to reduce app hangs (#…

82fe38a

…15774)" This reverts commit 0bebc05, reversing changes made to b7e7ffd.

itsmeichigo mentioned this pull request Jun 24, 2025

Revert introduction of lightweight objects for safety #15807

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add option to fetch lightweight objects to reduce app hangs #15774

Add option to fetch lightweight objects to reduce app hangs #15774

itsmeichigo commented Jun 18, 2025 •

edited

Loading

Uh oh!

wpmobilebot commented Jun 18, 2025 •

edited

Loading

Uh oh!

itsmeichigo commented Jun 19, 2025 •

edited

Loading

Uh oh!

RafaelKayumov left a comment

Uh oh!

itsmeichigo commented Jun 20, 2025 •

edited

Loading

Uh oh!

Uh oh!

joshheald commented Jun 24, 2025

Uh oh!

itsmeichigo commented Jun 24, 2025 •

edited

Loading

Uh oh!

joshheald commented Jun 24, 2025

Uh oh!

jaclync commented Jun 24, 2025

Uh oh!

Uh oh!

Add option to fetch lightweight objects to reduce app hangs #15774

Add option to fetch lightweight objects to reduce app hangs #15774

Conversation

itsmeichigo commented Jun 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Testing steps

TC1: Loading order details

TC2: Shipping Labels

TC3: Test order entrypoint

TC4: Blaze campaign creation form

TC5: Dashboard order count

TC6: Product list

Testing information

Screenshots

Uh oh!

wpmobilebot commented Jun 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

itsmeichigo commented Jun 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RafaelKayumov left a comment

Choose a reason for hiding this comment

Mostly I see it in Order List tab.

Uh oh!

itsmeichigo commented Jun 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

joshheald commented Jun 24, 2025

Uh oh!

itsmeichigo commented Jun 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joshheald commented Jun 24, 2025

Uh oh!

jaclync commented Jun 24, 2025

Uh oh!

Uh oh!

itsmeichigo commented Jun 18, 2025 •

edited

Loading

wpmobilebot commented Jun 18, 2025 •

edited

Loading

itsmeichigo commented Jun 19, 2025 •

edited

Loading

itsmeichigo commented Jun 20, 2025 •

edited

Loading

itsmeichigo commented Jun 24, 2025 •

edited

Loading